Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

magnorm fix #98

Merged
merged 2 commits into from
May 14, 2024
Merged

magnorm fix #98

merged 2 commits into from
May 14, 2024

Conversation

jchiang87
Copy link
Collaborator

This is a more reliable way to set the SED normalization using the magnorm value, and it also fixes the mismatch between flux calculations for stars when using galsim 2.4.8 vs 2.5.1+.

I also simplified the BaseObject._get_sed interface since passing back magnorm is no longer needed (and in most cases, that value was being ignored anyway).

Copy link
Collaborator

@JoanneBogart JoanneBogart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not necessarily requesting changes - just an explanation (see inline comment)
Otherwise it looks fine. I never liked the old interface for _get_sed

"""
# Compute the flux density from magnorm in units of erg/cm^2/s/nm.
fnu = (magnorm * u.ABmag).to_value(u.erg/u.s/u.cm**2/u.Hz)
flambda = fnu * (astropy.constants.c/wl**2).to_value(u.Hz/u.nm)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is wl squared here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because flambda = fnu * (nu / wl) and nu = c / wl.

Copy link
Collaborator

@JoanneBogart JoanneBogart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jchiang87 jchiang87 merged commit 95eb4b6 into main May 14, 2024
1 check passed
@jchiang87 jchiang87 deleted the u/jchiang/magnorm_fix branch May 14, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants